Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mavenview #273

Closed
wants to merge 3 commits into from
Closed

Mavenview #273

wants to merge 3 commits into from

Conversation

nickman
Copy link
Contributor

@nickman nickman commented Jan 25, 2014

This is a modified implementation of this mavenization proposal https://groups.google.com/forum/#!topic/opentsdb/lQQgU6hf6rU.

The basics are:

  1. The shell file maven-view.sh which creates a subdirectory in the opentsdb root called mavenview and then builds a mavenized view of the project using symbolic links. (Obviously this will not work for Windows). Changes and additions in the maven view will be reflected in the root project (in all but src/tsd). The mavenview directory can then be used for fully functional maven building (of the FatJar), running unit tests and supporting IDE projects like eclipse and intellij. No changes occur in the root when running this shell file. The shell file can be run in the project root as ./maven-view.sh ..
  2. The pom file maven-view.pom.xml is copied into the mavenview root as pom.xml when the shell file is run.
  3. Added mavenview to .gitignore, since there are no SCM controlled artifacts in there.

Running unit tests in the mavenview created a couple of issues, addressed with the following changes in src/test:

  1. A new test utility class called PluginJARFactory auto-generates the plugin jar used for testing. This appears duplicative in the code right now but I feel this will simplify and offer more flexible options for generating dynamic test plugin jars. The utility implementations look like this:
  /**
   * Creates the plugin jar
   */
  @BeforeClass
  public static void initPluginJar() {
      PluginJARFactory.newBuilder("plugin_test.jar")
        .service("net.opentsdb.plugin.DummyPlugin", "net.opentsdb.plugin.DummyPluginA", "net.opentsdb.plugin.DummyPluginB")
        .service("net.opentsdb.search.SearchPlugin", "net.opentsdb.search.DummySearchPlugin")
        .service("net.opentsdb.tsd.HttpSerializer", "net.opentsdb.tsd.DummyHttpSerializer")
        .service("net.opentsdb.tsd.RpcPlugin", "net.opentsdb.tsd.DummyRpcPlugin")
        .service("net.opentsdb.tsd.RTPublisher", "net.opentsdb.tsd.DummyRTPublisher")
        .build();
  }

  /**
   * Clears the created plugin jar
   */
  @AfterClass
  public static void clearPluginJar() {
      PluginJARFactory.purge();
  }

Currently 18 out of 1362 unit tests are failing, mostly because mygnuplot.sh is not in the CLASSPATH. I propose that this be addressed as part of making the FatJar executable ( issue #272 ).

In two cases, the @PowerMockIgnore annotation had to be updated to add a new package.

@PowerMockIgnore({"javax.management.*", "javax.xml.*",
    "ch.qos.*", "org.slf4j.*",
    "com.sum.*", "org.xml.*", "com.sun.org.apache.xerces.internal.jaxp.*"})

@manolama
Copy link
Member

Oh man, thank you SO much for the maven plugin fix!

So would this replace the "build.sh pom" command entirely?

@nickman
Copy link
Contributor Author

nickman commented Feb 26, 2014

It does, except it doesn't implement the maven-gpg-plugin, but I was not sure if anyone actually used this. Obviously it would be easy to add.

Two Quick things:

  1. There's some weirdness in the net.opentsdb.tsd.client package, even with the current flattened out source tree because the source declares the package to be "tsd.client". If we could correct the package name (to "net.opentsdb.tsd.client") in the source, it would clean up the mavenview. Ideally, the symbolic links would only be for directories, where as the tsd.client files require individual file links. Not a biggie, but it would help. (I did not make that change myself on account of tsuna's expressed desire not to move around source, but a flat source tree is one thing.... wrong package names is another......)
  2. I need to update the mavenview pom for the hbase 0.96 dependencies. They're not in there now. (Oh, and I volunteer to keep it up to date....)

Let me know if you need me to make either of those mods to my fork for the pull request.

Cheers.

//Nicholas

@nickman
Copy link
Contributor Author

nickman commented Feb 26, 2014

And it also occurred to me that the maven-view.sh execution currently prompts the user to continue. This should probably have an override for automated build environments (like Jenkins et. al.) so the code can be cloned, mavenized, built and tested automatically.

Let me know if you want this to go in the fork.

@manolama
Copy link
Member

manolama commented Apr 9, 2014

Hi Nick, could you take a look at #303? I tried it against next and it worked well to create the sym links for IDEs and run the unit tests with the plugins under Maven. Will it work for the FatJar compilation?

@nickman
Copy link
Contributor Author

nickman commented Apr 9, 2014

Will do. Need 24 hrs.
On Apr 9, 2014 1:14 PM, "Chris Larsen" notifications@github.com wrote:

Hi Nick, could you take a look at #303#303?
I tried it against next and it worked well to create the sym links for IDEs
and run the unit tests with the plugins under Maven. Will it work for the
FatJar compilation?


Reply to this email directly or view it on GitHubhttps://github.com//pull/273#issuecomment-39990528
.

@nickman
Copy link
Contributor Author

nickman commented Apr 9, 2014

Chris;

Have not tried it yet, but it looks good.
I think the best approach would be to take #303's modified pom template (pom.xml.in) and extend it with the FatJar modifications, but basically, we need to merge the two mod sets. I can reach out to jesse5e and discuss, unless you have a specific tactic you want followed.

//Nicholas

@manolama
Copy link
Member

manolama commented Apr 9, 2014

Awesome, thanks Nick. I'll merge #303 then for 2.0 so the unit tests work correctly. Then you can work with @jesse5e and extend the pom for FatJar.

@johann8384
Copy link
Member

This is all merged into 3.0 right? So we can close this PR?

@manolama
Copy link
Member

Yeah we have Maven in 3.0 so we'll close this one for now. Thanks!

@manolama manolama closed this Jun 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants